Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/awsxray] Adjust AwsXRay segment conversion logic #33000

Conversation

jj22ee
Copy link
Contributor

@jj22ee jj22ee commented May 11, 2024

Description:

Cherry-picking from downstream:
amazon-contributing#111
amazon-contributing#115
amazon-contributing#127

  • We are adjusting the segment creation to accommodate local root spans.
    If a span is a not a local root, then we keep existing behavior.
    If it is a local root then:
    • If it is an Internal or Server span, then promote it to a segment.
      Else we will split it into a segment and subsegment. The segment will represent the service operation and the subsegment will represent the dependency (service A calls service B).
  • Update the common logic for setting segment.Name, which previously only looked at CLIENT/PRODUCER spans, to also look at CONSUMER spans.

Link to tracking Issue:

Testing:
Unit Testing

Documentation:

atshaw43 and others added 4 commits May 10, 2024 20:59
In this commit, we are fixing a couple small bugs missed in the previous review. First, we are adding a nil-guard when settting dependencySubsegment.Name, to avoid the (unlikely, but possible) scenario where local root dependency spans are created without awsRemoteService. Second, we are updating the common logic for setting segment.Name, which previously only looked at CLIENT/PRODUCER spans, but now needs to look at CONSUMER spans.
@jj22ee jj22ee marked this pull request as ready for review May 16, 2024 17:31
jknollmeyer and others added 2 commits May 16, 2024 12:08
* Add logic for stripping the AWS.SDK prefix for Local Root spans

- Previous change to strip the prefix in other cases open-telemetry@7c037ad

* De-duplicate identical logic
Copy link
Contributor

@fatsheep9146 fatsheep9146 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit

@fatsheep9146 fatsheep9146 added the ready to merge Code review completed; ready to merge by maintainers label May 17, 2024
@andrzej-stencel andrzej-stencel merged commit 902d846 into open-telemetry:main May 22, 2024
162 checks passed
@github-actions github-actions bot added this to the next release milestone May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/awsxray ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants